Skip to content

[PM-40493] Claimed Domain Account Delete Check - #8263

Merged
JaredSnider-Bitwarden merged 3 commits into
mainfrom
auth/pm-40493/claimed-domain-account-delete-check
Aug 28, 2026
Merged

[PM-40493] Claimed Domain Account Delete Check#8263
JaredSnider-Bitwarden merged 3 commits into
mainfrom
auth/pm-40493/claimed-domain-account-delete-check

Conversation

@rr-bw

@rr-bw rr-bw commented Aug 26, 2026 •
edited by atlassian Bot
Loading

Copy link
Copy Markdown
Contributor

🎟️ Tracking

PM-40493

📔 Objective

Adds a claimed-domain check to DeleteAsync({single param}), so that both endpoints are covered by the one check:

  • DELETE /accounts - Delete() (was already covered)
  • POST /accounts/delete-recover-token - PostDeleteRecoverToken()

📸 Screenshots

@rr-bw
rr-bw marked this pull request as ready for review August 26, 2026 19:53
@rr-bw
rr-bw requested a review from a team as a code owner August 26, 2026 19:53
@github-actions

github-actions Bot commented Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR moves the claimed-domain guard from AccountsController.Delete into UserService.DeleteAsync(User), so both the authenticated DELETE /accounts path and the anonymous POST /accounts/delete-recover-token path are covered by a single check. I traced every caller of IUserService.DeleteAsync / UserManager<User>.DeleteAsync across src, util, and bitwarden_license — the only two are the controller actions named in the description, and the org-admin path (DeleteClaimedOrganizationUserAccountCommand) deletes through IUserRepository.DeleteManyAsync, so it is unaffected. In the token overload the guard runs after VerifyUserTokenAsync, which keeps the claimed/unclaimed state from being observable without a valid delete token. The error surface for DELETE /accounts is unchanged, since the controller previously threw the same BadRequestException(new CannotDeleteClaimedAccountError().Message).

Code Review Details

No findings.

JaredSnider-Bitwarden
JaredSnider-Bitwarden approved these changes Aug 27, 2026
View reviewed changes

@JaredSnider-Bitwarden JaredSnider-Bitwarden left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice work!

@codecov

codecov Bot commented Aug 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.66%. Comparing base (7640e4b) to head (55ea4b3).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8263      +/-   ##
==========================================
+ Coverage   63.64%   63.66%   +0.02%     
==========================================
  Files        2430     2430              
  Lines      104962   104962              
  Branches     9506     9506              
==========================================
+ Hits        66798    66822      +24     
+ Misses      35868    35844      -24     
  Partials     2296     2296

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JaredSnider-Bitwarden JaredSnider-Bitwarden added the t:bugfix Change Type - Bugfix label Aug 28, 2026
@JaredSnider-Bitwarden
JaredSnider-Bitwarden merged commit 25f1f9c into main Aug 28, 2026
46 of 47 checks passed
@JaredSnider-Bitwarden
JaredSnider-Bitwarden deleted the auth/pm-40493/claimed-domain-account-delete-check branch August 28, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JaredSnider-Bitwarden JaredSnider-Bitwarden JaredSnider-Bitwarden approved these changes

Assignees

No one assigned

Labels

t:bugfix Change Type - Bugfix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rr-bw @JaredSnider-Bitwarden